feat(trustify): migrate DA backend from Trustify v2 to v3 endpoints#638
feat(trustify): migrate DA backend from Trustify v2 to v3 endpoints#638ruromero wants to merge 7 commits into
Conversation
Reviewer's GuideMigrates DA backend Trustify integration from v2 to v3 analyze/recommend endpoints and rewrites response handling logic and tests to consume the new v3 response shape (base_score, purl_statuses, advisory.issuer, remediations, version ranges). Sequence diagram for Trustify v3 analyze endpoint integrationsequenceDiagram
participant DAService
participant TrustifyApi
participant TrustifyResponseHandler
participant Issue
DAService->>TrustifyApi: POST /api/v3/vulnerability/analyze
TrustifyApi-->>DAService: v3 response (details, purl_statuses, base_score)
DAService->>TrustifyResponseHandler: toIssues(response)
TrustifyResponseHandler->>TrustifyResponseHandler: getSource(purl_status)
TrustifyResponseHandler->>TrustifyResponseHandler: setCvssData(issue, vuln, purlStatus)
TrustifyResponseHandler->>Issue: cvssScore(base_score.score)
TrustifyResponseHandler->>Issue: setSeverity(SeverityUtils.fromValue | fromScore)
TrustifyResponseHandler->>Issue: setRemediation(Remediation)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The new
setCvssDatalogic relies solely onbase_score; if Trustify ever omits this field for some entries, consider a fallback to per-advisory/purl_status scoring data to avoid silently producing issues without CVSS information. - The remediation construction in
setCvssData(version range + remediations) has become quite dense; consider extracting this into helper methods to make the transformation from v3 JSON toRemediationandVersionRangeeasier to follow and maintain. - Changing
DEFAULT_SOURCEfrom "manual" to "unknown" and deriving source fromissuer.nameinstead of the previous importer may affect how issues are grouped by source; if this is intentional, it may be worth centralizing the mapping logic in a dedicated helper to keep future adjustments localized.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `setCvssData` logic relies solely on `base_score`; if Trustify ever omits this field for some entries, consider a fallback to per-advisory/purl_status scoring data to avoid silently producing issues without CVSS information.
- The remediation construction in `setCvssData` (version range + remediations) has become quite dense; consider extracting this into helper methods to make the transformation from v3 JSON to `Remediation` and `VersionRange` easier to follow and maintain.
- Changing `DEFAULT_SOURCE` from "manual" to "unknown" and deriving source from `issuer.name` instead of the previous importer may affect how issues are grouped by source; if this is intentional, it may be worth centralizing the mapping logic in a dedicated helper to keep future adjustments localized.
## Individual Comments
### Comment 1
<location path="src/test/java/io/github/guacsec/trustifyda/integration/providers/trustify/TrustifyResponseHandlerTest.java" line_range="472" />
<code_context>
Issue issue = issues.get(0);
- // Should prioritize V4 based on SCORE_TYPE_ORDER
+ // v3 provides pre-computed base_score
assertEquals(7.2f, issue.getCvssScore());
assertEquals(Severity.HIGH, issue.getSeverity());
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test where base_score has a score but no severity to verify score-based severity fallback
The updated `setCvssData` falls back to `SeverityUtils.fromScore` when `base_score.severity` is missing or unparsable, but there’s no test for a `base_score` that has only a numeric score and no severity field. Please add a test (e.g., a variant of `testResponseToIssuesWithMultipleScoreTypes`) using such input to verify that severity is correctly derived from the score when Trustify omits `severity`.
Suggested implementation:
```java
List<Issue> issues = packageItem.issues();
Issue issue = issues.get(0);
// v3 provides pre-computed base_score
assertEquals(7.2f, issue.getCvssScore());
assertEquals(Severity.HIGH, issue.getSeverity());
}
@Test
void testBaseScoreSeverityFallbackWhenSeverityMissing() throws Exception {
String response = """
{
"vulnerabilities": [{
"identifier": "CVE-2024-1597",
"title": "Test CVE",
"scores": {
"cvss_v3": {
"version": "3.1",
"vector": "CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H",
"base_score": {
"score": 7.2
}
}
}
}],
"packages": [{
"purl": "pkg:maven/org.example/foo@1.0.0",
"issues": [{
"identifier": "CVE-2024-1597"
}]
}],
"warnings": []
}
""";
// Adapt this setup to however the existing tests obtain a PackageItem
PackageItem packageItem = trustifyResponseHandler.handleResponse(response);
List<Issue> issuesWithScoreOnly = packageItem.issues();
Issue issueWithScoreOnly = issuesWithScoreOnly.get(0);
// base_score has only a numeric score and no severity; severity should be derived from the score
assertEquals(7.2f, issueWithScoreOnly.getCvssScore());
assertEquals(Severity.HIGH, issueWithScoreOnly.getSeverity());
}
{
"identifier": "CVE-2024-1597",
```
The new test assumes there is a `trustifyResponseHandler.handleResponse(String)` (or similar) method that returns a `PackageItem`, consistent with how `packageItem` is created in the existing tests. To integrate this test correctly, you should:
1. Replace `trustifyResponseHandler.handleResponse(response)` with the exact helper or setup logic used in the other tests in this class to obtain `packageItem` from the JSON response.
2. Ensure the JSON structure (particularly the `packages` and `issues` linkage) matches what the existing tests expect. If the existing tests use a slightly different envelope (e.g., different top-level keys or nesting), adjust the JSON in the new test to mirror that structure while keeping `base_score` with only a `score` field and no `severity`.
3. If the class uses a different naming convention for the handler or `PackageItem`, adjust the types and variable names accordingly.
</issue_to_address>
### Comment 2
<location path="src/test/java/io/github/guacsec/trustifyda/integration/providers/trustify/TrustifyResponseHandlerTest.java" line_range="699" />
<code_context>
Issue issue = issues.get(0);
- assertEquals("manual", issue.getSource());
+ assertEquals("unknown", issue.getSource());
}
</code_context>
<issue_to_address>
**suggestion (testing):** Consider a dedicated test for purl_status entries without an advisory to exercise getSource’s DEFAULT_SOURCE path explicitly
Since `getSource` now derives the source from `advisory.issuer.name` with a fallback to "unknown", this test only covers the `issuer == null` case when an advisory is present. Please also add a test where `purl_status` has no `advisory` at all to cover the `advisory == null` branch and verify it still returns "unknown" without throwing.
Suggested implementation:
```java
import io.github.guacsec.trustifyda.api.PackageRef;
import io.github.guacsec.trustifyda.api.v5.Issue;
import io.github.guacsec.trustifyda.api.v5.RemediationCategory;
import io.github.guacsec.trustifyda.api.v5.Severity;
import io.github.guacsec.trustifyda.integration.Constants;
import io.github.guacsec.trustifyda.integration.providers.trustify.ubi.UBIRecommendation;
@Test
void testIssueSourceIsUnknownWhenAdvisoryIsMissing() {
// Response containing a purl_status entry without an advisory block to
// exercise the getSource DEFAULT_SOURCE ("unknown") path.
String responseWithoutAdvisory = """
{
"purl_status": [
{
"purl": "pkg:maven/org.example/foo@1.0.0",
"status": "vulnerable"
}
]
}
""";
List<Issue> issues = TrustifyResponseHandler.responseToIssues(responseWithoutAdvisory);
assertEquals(1, issues.size());
Issue issue = issues.get(0);
assertEquals("unknown", issue.getSource());
}
private static Stream<String> testResponseToIssuesWithValidData() {
return Stream.of(
```
Depending on the existing code in `TrustifyResponseHandlerTest` and `TrustifyResponseHandler`, you may need to:
1. Ensure the following imports are present at the top of the file (if they are not already):
- `import java.util.List;`
- `import org.junit.jupiter.api.Test;`
2. Adjust the call `TrustifyResponseHandler.responseToIssues(responseWithoutAdvisory);` if your handler uses a different factory/utility method or signature (for example `fromResponse`, `toIssues`, or an instance method).
3. If your Trustify JSON structure requires additional mandatory fields beyond `purl` and `status`, extend the `responseWithoutAdvisory` JSON fixture accordingly while keeping the `advisory` field absent for this test case.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| Issue issue = issues.get(0); | ||
|
|
||
| // Should prioritize V4 based on SCORE_TYPE_ORDER | ||
| // v3 provides pre-computed base_score |
There was a problem hiding this comment.
suggestion (testing): Consider adding a test where base_score has a score but no severity to verify score-based severity fallback
The updated setCvssData falls back to SeverityUtils.fromScore when base_score.severity is missing or unparsable, but there’s no test for a base_score that has only a numeric score and no severity field. Please add a test (e.g., a variant of testResponseToIssuesWithMultipleScoreTypes) using such input to verify that severity is correctly derived from the score when Trustify omits severity.
Suggested implementation:
List<Issue> issues = packageItem.issues();
Issue issue = issues.get(0);
// v3 provides pre-computed base_score
assertEquals(7.2f, issue.getCvssScore());
assertEquals(Severity.HIGH, issue.getSeverity());
}
@Test
void testBaseScoreSeverityFallbackWhenSeverityMissing() throws Exception {
String response = """
{
"vulnerabilities": [{
"identifier": "CVE-2024-1597",
"title": "Test CVE",
"scores": {
"cvss_v3": {
"version": "3.1",
"vector": "CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H",
"base_score": {
"score": 7.2
}
}
}
}],
"packages": [{
"purl": "pkg:maven/org.example/foo@1.0.0",
"issues": [{
"identifier": "CVE-2024-1597"
}]
}],
"warnings": []
}
""";
// Adapt this setup to however the existing tests obtain a PackageItem
PackageItem packageItem = trustifyResponseHandler.handleResponse(response);
List<Issue> issuesWithScoreOnly = packageItem.issues();
Issue issueWithScoreOnly = issuesWithScoreOnly.get(0);
// base_score has only a numeric score and no severity; severity should be derived from the score
assertEquals(7.2f, issueWithScoreOnly.getCvssScore());
assertEquals(Severity.HIGH, issueWithScoreOnly.getSeverity());
}
{
"identifier": "CVE-2024-1597",The new test assumes there is a trustifyResponseHandler.handleResponse(String) (or similar) method that returns a PackageItem, consistent with how packageItem is created in the existing tests. To integrate this test correctly, you should:
- Replace
trustifyResponseHandler.handleResponse(response)with the exact helper or setup logic used in the other tests in this class to obtainpackageItemfrom the JSON response. - Ensure the JSON structure (particularly the
packagesandissueslinkage) matches what the existing tests expect. If the existing tests use a slightly different envelope (e.g., different top-level keys or nesting), adjust the JSON in the new test to mirror that structure while keepingbase_scorewith only ascorefield and noseverity. - If the class uses a different naming convention for the handler or
PackageItem, adjust the types and variable names accordingly.
|
|
||
| Issue issue = issues.get(0); | ||
| assertEquals("manual", issue.getSource()); | ||
| assertEquals("unknown", issue.getSource()); |
There was a problem hiding this comment.
suggestion (testing): Consider a dedicated test for purl_status entries without an advisory to exercise getSource’s DEFAULT_SOURCE path explicitly
Since getSource now derives the source from advisory.issuer.name with a fallback to "unknown", this test only covers the issuer == null case when an advisory is present. Please also add a test where purl_status has no advisory at all to cover the advisory == null branch and verify it still returns "unknown" without throwing.
Suggested implementation:
import io.github.guacsec.trustifyda.api.PackageRef;
import io.github.guacsec.trustifyda.api.v5.Issue;
import io.github.guacsec.trustifyda.api.v5.RemediationCategory;
import io.github.guacsec.trustifyda.api.v5.Severity;
import io.github.guacsec.trustifyda.integration.Constants;
import io.github.guacsec.trustifyda.integration.providers.trustify.ubi.UBIRecommendation;
@Test
void testIssueSourceIsUnknownWhenAdvisoryIsMissing() {
// Response containing a purl_status entry without an advisory block to
// exercise the getSource DEFAULT_SOURCE ("unknown") path.
String responseWithoutAdvisory = """
{
"purl_status": [
{
"purl": "pkg:maven/org.example/foo@1.0.0",
"status": "vulnerable"
}
]
}
""";
List<Issue> issues = TrustifyResponseHandler.responseToIssues(responseWithoutAdvisory);
assertEquals(1, issues.size());
Issue issue = issues.get(0);
assertEquals("unknown", issue.getSource());
}
private static Stream<String> testResponseToIssuesWithValidData() {
return Stream.of(Depending on the existing code in TrustifyResponseHandlerTest and TrustifyResponseHandler, you may need to:
- Ensure the following imports are present at the top of the file (if they are not already):
import java.util.List;import org.junit.jupiter.api.Test;
- Adjust the call
TrustifyResponseHandler.responseToIssues(responseWithoutAdvisory);if your handler uses a different factory/utility method or signature (for examplefromResponse,toIssues, or an instance method). - If your Trustify JSON structure requires additional mandatory fields beyond
purlandstatus, extend theresponseWithoutAdvisoryJSON fixture accordingly while keeping theadvisoryfield absent for this test case.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #638 +/- ##
============================================
- Coverage 56.86% 56.67% -0.20%
- Complexity 823 836 +13
============================================
Files 94 92 -2
Lines 4815 4863 +48
Branches 624 655 +31
============================================
+ Hits 2738 2756 +18
- Misses 1802 1814 +12
- Partials 275 293 +18
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
57e5ce8 to
0f446ab
Compare
Verification Report for TC-4522 (commit a007ebb)
Overall: WARNTwo warnings to note:
No blocking issues found. All acceptance criteria are met and CI passes. This comment was AI-generated by sdlc-workflow/verify-pr v0.11.0. |
a-oren
left a comment
There was a problem hiding this comment.
Looks great, just deleting two files that are no longer in use.
| import io.github.guacsec.trustifyda.model.trustify.AdvisoryScore; | ||
| import io.github.guacsec.trustifyda.model.trustify.ScoreType; |
There was a problem hiding this comment.
AdvisoryScore.java and ScoreType.java are no longer imported or used anywhere in the codebase after this PR, delete both files
Cover untested code paths in setCvssData, getSource, and toIssues: fallback to purlStatus scores, full version range fields, remediation URL, unknown remediation category, missing/blank advisory source, score-based severity derivation, and CVE deduplication logic. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…stedContent RecommendationAggregation and RegistryEnrichmentService were overwriting the Remediation object when adding trustedContent, destroying any upstream fixedIn/versionRanges/remediations data from Trustify v3. Now merges onto the existing Remediation instead. Also updates the remediation counter in ProviderResponseHandler to count upstream-only remediations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When highInclusive is true, the high version is still affected — it should not appear in fixedIn. Read highInclusive before deciding whether to populate the fixed version list. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
TC-4522 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Migrates the DA backend from Trustify v2
/api/v2/vulnerability/analyzeand/api/v2/purl/recommendendpoints to v3/api/v3/vulnerability/analyzeand/api/v3/purl/recommend.TrustifyResponseHandlerto parse the v3 response format (details[].purl_statuses[],base_score,advisory.issuer)trustify-da-api-modelto2.0.10-SNAPSHOTTest plan
mvn verify -Pdev)TrustifyResponseHandlerTest— 12 test cases cover: empty responses, single/multiple issues, severity mapping, remediation parsing, withdrawn advisories, CVE deduplication, unknown severity fallbackAnalysisTestintegration tests pass with v3 fixturesJira
TC-4522
Dependencies
Summary by Sourcery
Migrate Trustify integration to the v3 vulnerability and recommendation APIs and adapt response handling to the new schema.
New Features:
Enhancements:
Build:
Tests: